-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add single-commit
input
#78
base: main
Are you sure you want to change the base?
Conversation
Thanks @xaviergmail! I assume this option works fine with I think I'm going to tweak the docs a little bit - given that this is passed to JamesIves' action, it's a bit odd that our description of it would be more verbose than theirs. Given that I intend to add more parameters that are passed along directly (e.g. #32) I'll probably end up just adding a little list like "Parameters passed directly: single-commit, this, that, something-else". The warnings are appreciated but they're probably something that would be better contributed upstream. |
Looks like the answer is possibly no - logs from testing with
With
I'm thinking that It's really lucky in that case that It'd probably be better if, when
Problem with that approach is that force-pushing necessarily risks destroying work contributed by simultaneous workflows, even if we minimise it by pulling immediately before pushing - there's always going to be some amount of time in between. For reference, we do need to worry about simultaneous workflows, because merging a PR will dispatch the 'PR closed' event that removes the preview, and the 'push to main' event that starts a main deployment. I'm not convinced there's a way around this, honestly. If you want to replace/squash commits you need to rewrite history, if you want to rewrite history you need to force push, and if you want to force push you risk desyncs... right? I'd really love it if one of those assumptions wasn't true. EDIT: nedbat in #git on Libera.Chat has pointed me to git push --force-with-lease which solves exactly this problem! I might consider trying to implement this upstream at some point - I'm thinking a try-to-push loop, similar to the try-to-pull loop that |
You make good points around pr previews and main deployments! In our case, our dev/staging/prod deployments end up in S3 so I wasn't worried about conflicting deployments but I see the problem.
|
This change simply adds a configurable
single-commit
option to be passed toJamesIves/github-pages-deploy-action